Adversarial Testing Result: FAIL — 3 bugs found (1 security, 1 crash, 1 code correctness)

Scope: Full adversarial testing of Sandbox abstraction (base.py, local.py, docker.py, agent integration). Tested shell injection, path traversal, heredoc injection, concurrency, lifecycle edge cases, timeout cleanup, type safety, and contract verification.
Tests written: 67 (61 adversarial + 6 confirmation)
Tests passing: 63
Tests failing (findings): 4 (3 genuine bugs, 1 test infrastructure)

Finding 1: execute_code() language parameter injection

Category: Security Issue
Severity: Critical

Reproduction:

sandbox = LocalSandbox(working_dir="/tmp")
marker = "/tmp/injection_marker.txt"
malicious_language = f"touch {marker} #"
await sandbox._execute_code_to_result("print('safe')", language=malicious_language)
assert os.path.exists(marker)  # True — injection succeeded!

Observed behavior: The language parameter is interpolated directly into the shell command without sanitization: f"{language} -c {shlex.quote(code)}". Code is properly shlex.quote()'d, but language is NOT. Any caller passing untrusted input as language enables arbitrary command execution.

Expected behavior: language should be validated against an allowlist (e.g., python, ruby, node) or at minimum shlex.quote()'d.

Fix: Either shlex.quote(language) or validate against an allowlist:

# Option A: Quote it
async for chunk in self.execute(f"{shlex.quote(language)} -c {shlex.quote(code)}", timeout=timeout):
    yield chunk

# Option B: Allowlist (stricter)
ALLOWED_LANGUAGES = {"python", "python3", "ruby", "node", "bash", "sh"}
if language not in ALLOWED_LANGUAGES:
    raise ValueError(f"Unsupported language: {language}")

Finding 2: Long output lines crash readline()

Category: Bug
Severity: High

Reproduction:

sandbox = LocalSandbox(working_dir="/tmp")
await sandbox._execute_to_result("echo " + "A" * 100000)
# Raises: ValueError: Separator is found, but chunk is longer than limit

Observed behavior: asyncio.StreamReader.readline() has a default buffer limit of 64KB. When a command outputs a single line exceeding this, readline() raises ValueError('Separator is found, but chunk is longer than limit'). This propagates as an unhandled exception.

Expected behavior: The sandbox should handle long output lines gracefully. Either increase the buffer limit when creating the subprocess, or catch ValueError/LimitOverrunError in _read_stream.

Fix: Set a larger limit when creating the subprocess:

proc = await asyncio.create_subprocess_shell(
    command,
    cwd=self.working_dir,
    stdout=asyncio.subprocess.PIPE,
    stderr=asyncio.subprocess.PIPE,
    limit=1024 * 1024,  # 1MB line limit
)

Note: asyncio.create_subprocess_shell accepts a limit parameter that's passed to the StreamReader.

Finding 3: Non-existent working directory crashes

Category: Bug
Severity: Medium

Reproduction:

sandbox = LocalSandbox(working_dir="/nonexistent/path")
await sandbox._execute_to_result("echo hello")
# Raises: FileNotFoundError: [Errno 2] No such file or directory: '/nonexistent/path'

Observed behavior: asyncio.create_subprocess_shell(cwd="/nonexistent") raises FileNotFoundError which propagates unhandled.

Expected behavior: Either validate the working directory in __init__/start(), or catch the exception in execute() and return an ExecutionResult with a non-zero exit code and descriptive stderr.

Finding 4: proc.returncode or 0 masks None as 0

Category: Code Smell
Severity: Low

Location: local.py:115, docker.py:131, docker.py:262

Issue: The pattern proc.returncode or 0 uses Python's falsy semantics. If returncode is None (process not finished), it maps to 0 (success). The correct pattern is 0 if proc.returncode is None else proc.returncode.

Risk: Low in practice (after proc.wait(), returncode should never be None), but the code is semantically incorrect and fragile.

Other observations (non-findings)

✅ Heredoc injection: Base class write_file uses randomized STRANDS_EOF_{hex} delimiter — resistant to injection. LocalSandbox overrides with native file I/O (even better).

✅ DockerSandbox stdin piping: write_file uses stdin pipe instead of heredoc — immune to shell metacharacter injection.

✅ shlex.quote() on file paths: All path arguments are properly quoted.

✅ Concurrent execution: Multiple concurrent execute() calls work correctly.

✅ Context manager cleanup: Exception during async with properly calls stop().

✅ Docker container cleanup: stop() handles rm failures gracefully.

✅ Timeout process killing: Local and Docker sandboxes properly kill processes on timeout.

⚠️ Path traversal (LocalSandbox): write_file("../escape.txt") writes outside working directory. This is by design for LocalSandbox (host execution), but should be clearly documented as a security boundary. DockerSandbox containment handles this via container isolation.

Artifacts: All adversarial tests provided in the review above.

🤖 AI agent response. Strands Agents. Feedback welcome!